Change path max stack allocation to heap #10127
Open
+368
−113
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michal-shalev
force-pushed
the
change-path-max-stack-alloc-to-heap
branch
from
8cb5ec5
to
2149869
Compare
michal-shalev
changed the title
michal-shalev
force-pushed
the
change-path-max-stack-alloc-to-heap
branch
2 times, most recently
from
bbc428e
to
92fdb9a
Compare
michal-shalev
force-pushed
the
change-path-max-stack-alloc-to-heap
branch
from
92fdb9a
to
4920c1e
Compare
michal-shalev
force-pushed
the
change-path-max-stack-alloc-to-heap
branch
3 times, most recently
from
0dfae87
to
1bc0050
Compare
…er generating report
michal-shalev
force-pushed
the
change-path-max-stack-alloc-to-heap
branch
from
1bc0050
to
22cbe36
Compare
michal-shalev
force-pushed
the
change-path-max-stack-alloc-to-heap
branch
2 times, most recently
from
66bbe34
to
27e63b9
Compare
ivankochin
reviewed
Sep 18, 2024
Comment on lines
195
to
+197
| fuse_fd = fuse_open_channel(mountpoint, mountopts); | ||
| if (fuse_fd < 0) { | ||
| ret = fuse_fd; |
There was a problem hiding this comment.
Suggested change
| fuse_fd = fuse_open_channel(mountpoint, mountopts); | |
| if (fuse_fd < 0) { | |
| ret = fuse_fd; | |
| ret = fuse_open_channel(mountpoint, mountopts); | |
| if (ret< 0) { |
| } | ||
|
|
||
| /* Remove mount point directory */ | ||
| vfs_log("removing directory '%s'", mountpoint); | ||
| ret = rmdir(mountpoint); | ||
| if (ret < 0) { | ||
| vfs_error("failed to remove directory '%s': %m", mountpoint); | ||
| return ret; | ||
| goto out_free_mountpoint; |
| @@ -1778,6 +1795,11 @@ void ucs_config_parse_config_files() | |||
|
|
|||
| /* Current working dir */ | |||
| ucs_config_parse_config_file(".", UCX_CONFIG_FILE_NAME, 1); | |||
|
|
|||
| out_free_path: | |||
There was a problem hiding this comment.
Unused label (pls check other places too)?
| @@ -433,10 +433,10 @@ void ucs_memtrack_init() | |||
| return; | |||
| } | |||
|
|
|||
| ucs_memtrack_vfs_init(); | |||
There was a problem hiding this comment.
Is that change related to this PR?
There was a problem hiding this comment.
yes, I changed allocation at VFS to heap and it revealed this issue - an invalid count in the memtrack (tests were failing)
|
|
||
| /* Build parent nodes along the rel_path, without associated object */ | ||
| next_token = rel_path_buf; | ||
| token = strsep(&next_token, "/"); | ||
| while (next_token != NULL) { | ||
| current_node = ucs_vfs_node_add_subdir(current_node, token); | ||
| if (current_node == NULL) { | ||
| return UCS_ERR_NO_MEMORY; | ||
| status = UCS_ERR_NO_MEMORY; | ||
| goto out_free_abs_path_buf; | ||
| } | ||
|
|
||
| token = strsep(&next_token, "/"); | ||
| } | ||
|
|
||
| ucs_vfs_node_build_path(current_node, token, abs_path_buf, |
There was a problem hiding this comment.
Suggested change
| ucs_vfs_node_build_path(current_node, token, abs_path_buf, | |
| ucs_vfs_node_build_path(current_node, token, abs_path_buf, PATH_MAX); |
| ret = mkdir(dirname(sock_path_dir), S_IRWXU); | ||
| if ((ret < 0) && (errno != EEXIST)) { | ||
| ucs_log(log_level, "failed to create directory '%s': %m", | ||
| sock_path_dir); | ||
| return -errno; | ||
| ret = -errno; | ||
| goto out_free_sock_path_dir; |
michal-shalev
changed the title
michal-shalev
changed the title
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR changes all stack allocations of char arrays sized
PATH_MAXto heap allocations.Why?
Customers with limited stack sizes have recently experienced stack overflow issues when running UCX. Moving these allocations to the heap prevents such overflows.
How?
Heap allocations replace stack allocations for char arrays of size
PATH_MAX.